Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2382 some incorrect usage related to the recent shell=false issue #2417

Merged

Conversation

bmaltais
Copy link
Owner

Attempt to implement a shell=False model that hopefully will properly address running on different platform without "fine not found" issues like some users had reported in the past.

When shell=False, we should use a sequence instead of a string. This is because it behaves differently on different platforms, and using a sequence should work on both Windows and Linux.
https://stackoverflow.com/a/30830867/8706033

For a complete explanation, please refer to the python documentation:
https://docs.python.org/3/library/subprocess.html#popen-constructor

I have extracted the key points as follows:

args should be a sequence of program arguments or else a single string or path-like object. By default, the program to execute is the first item in args if args is a sequence. If args is a string, the interpretation is platform-dependent and described below. See the shell and executable arguments for additional differences from the default behavior.
Unless otherwise stated, it is recommended to pass args as a sequence.

On POSIX, if args is a string, the string is interpreted as the name or path of the program to execute.
However, this can only be done if not passing arguments to the program.

On Windows, if args is a sequence, it will be converted to a string in a manner described in Converting an argument sequence to a string on Windows. This is because the underlying CreateProcess() operates on strings.

@bmaltais bmaltais merged commit 7bbc99d into dev Apr 29, 2024
2 checks passed
@bmaltais bmaltais deleted the 2382-some-incorrect-usage-related-to-the-recent-shell=false-issue branch April 29, 2024 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant